Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use symmetric return path for non-VPC traffic - alternate solution #1475

Merged
merged 6 commits into from
Jun 2, 2021
Merged

Use symmetric return path for non-VPC traffic - alternate solution #1475

merged 6 commits into from
Jun 2, 2021

Conversation

kishorj
Copy link
Contributor

@kishorj kishorj commented May 26, 2021 •
edited
Loading

What type of PR is this?
bug

Which issue does this PR fix:
Fixes #1392, #1423

What does this PR do / Why do we need it:
When external SNAT is disabled, this PR does the following

  • setup iptables rules to mark egress traffic from pods that are destined to external network
  • external traffic will be sent through the primary interface

This is an alternate fix. It doesn't require permissive rp filter configuration on the secondary ENIs, or a new connmark.

If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:
N/A

Testing done on this change:

  • When external SNAT is enabled, the iptables conmark rules added to the nat table get cleaned up

  • With External SNAT disabled

    • connmark rules get added to the nat table
    • pods are able to initiate connections to hosts outside the VPC, traffic is through the primary interface
    • in case of NLB-IP, verify traffic flows as expected for client IP preservation enabled/disabled, pods on primary/secondary interfaces
    • UDP traffic works fine
    • instance mode NLB works as expected - verified externalTrafficPolicy Cluster, Local, IP preservation enabled/disabled, pods on primary/secondary interfaces
    • in-cluster traffic works as expected
    • enabled security group per pod, verified pods can communicate within the cluster, pods can receive client IP preserved traffic from the NLB

  • With External SNAT enabled

    • connmark and SNAT rules get removed from the nat table
    • instance mode NLB works fine with IP preserve enabled/disabled, pods on primary/secondary interfaces
    • IP mode NLB works as expected with IP preserve enabled/disabled, pods on primary/secondary interfaces
    • pod can communicate to hosts within or outside VPC
    • enabled security group per pod, verified traffic flows

  • New integration tests pass after some modification

Automation added to e2e:

Will this break upgrades or downgrades. Has updating a running cluster been tested?:
No impact on upgrade/downgrade. On, downgrade conn mark rules in the nat table remain until node is rebooted, but there is no impact on the functionality.

Does this change require updates to the CNI daemonset config files to work?:

Does this PR introduce any user-facing change?:

Fixes the issue with non-VPC traffic ingress from secondary ENIs when external SNAT feature is not used.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@kishorj kishorj changed the title Use symmetric return path for non-VPC traffic Use symmetric return path for non-VPC traffic - alternate solution May 26, 2021
@kishorj kishorj marked this pull request as ready for review May 27, 2021 00:39
anguslees
anguslees approved these changes May 27, 2021
View reviewed changes
pkg/networkutils/network_test.go Outdated Show resolved Hide resolved
This was referenced May 28, 2021
Merged
Closed
kishorj
kishorj commented May 28, 2021
View reviewed changes
pkg/networkutils/network.go Outdated Show resolved Hide resolved
kishorj added a commit that referenced this pull request May 28, 2021
@kishorj
remove validation of VPC CIDRs from ip rules (#1476)
66180f4 
Recent changes in PR #1475 removes the VPC CIDR ranges from
the ip route rules. Modify the pod networking agent for compatibility
with the new changes introduced for symmetric return path.
achevuru
achevuru reviewed Jun 1, 2021
View reviewed changes
README.md Outdated Show resolved Hide resolved
@jayanthvn
Copy link
Contributor

Lets wait for the integration tests to finish.

@jayanthvn jayanthvn merged commit a07c423 into aws:master Jun 2, 2021
M00nF1sh pushed a commit to M00nF1sh/amazon-vpc-cni-k8s that referenced this pull request Jun 7, 2021
@kishorj @M00nF1sh
remove validation of VPC CIDRs from ip rules (aws#1476)
d6b3e0d 
Recent changes in PR aws#1475 removes the VPC CIDR ranges from
the ip route rules. Modify the pod networking agent for compatibility
with the new changes introduced for symmetric return path.
M00nF1sh pushed a commit to M00nF1sh/amazon-vpc-cni-k8s that referenced this pull request Jun 7, 2021
@kishorj @M00nF1sh
Use symmetric return path for non-VPC traffic - alternate solution (a…
ae12798 
…ws#1475)

* use symmetric return path for non-VPC traffic

* account for custom veth prefix configuration

* update host iptables rules on VPC CIDR change

* update integration tests to recognize new changes

* new integration test: reset aws-node config

* update README
M00nF1sh pushed a commit to M00nF1sh/amazon-vpc-cni-k8s that referenced this pull request Jun 7, 2021
@kishorj @M00nF1sh
Use symmetric return path for non-VPC traffic - alternate solution (a…
2198441 
…ws#1475)

* use symmetric return path for non-VPC traffic

* account for custom veth prefix configuration

* update host iptables rules on VPC CIDR change

* update integration tests to recognize new changes

* new integration test: reset aws-node config

* update README
M00nF1sh added a commit that referenced this pull request Jun 7, 2021
@M00nF1sh @kishorj
remove validation of VPC CIDRs from ip rules (#1476) (#1493)
48c4a42 
Recent changes in PR #1475 removes the VPC CIDR ranges from
the ip route rules. Modify the pod networking agent for compatibility
with the new changes introduced for symmetric return path.

Co-authored-by: Kishor Joshi <joshikis@amazon.com>
M00nF1sh added a commit that referenced this pull request Jun 7, 2021
@M00nF1sh @kishorj
Use symmetric return path for non-VPC traffic - alternate solution (#…
45bcab5 
…1475) (#1494)

* use symmetric return path for non-VPC traffic

* account for custom veth prefix configuration

* update host iptables rules on VPC CIDR change

* update integration tests to recognize new changes

* new integration test: reset aws-node config

* update README

Co-authored-by: Kishor Joshi <joshikis@amazon.com>
@kishorj kishorj deleted the outbound-connmark branch June 8, 2021 20:54
This was referenced Jun 10, 2021
Closed
Merged
@Nuru Nuru mentioned this pull request Jul 14, 2021
Closed
@achevuru achevuru mentioned this pull request Feb 9, 2022
Closed
a2ush added a commit to a2ush/amazon-vpc-cni-k8s that referenced this pull request Mar 12, 2022
@a2ush
Modify the example comment along the current behavior.
566bcf7 
From v1.8.0, amazon-vpc-cni-k8s no longer sets VPC CIDR to the from-pod rule.
aws#1475
Ex)
0:	from all lookup local
512:	from all to 192.168.13.238 lookup main
512:	from all to 192.168.16.73 lookup main
512:	from all to 192.168.17.61 lookup main
512:	from all to 192.168.23.14 lookup main
512:	from all to 192.168.25.98 lookup main
512:	from all to 192.168.31.149 lookup main
512:	from all to 192.168.12.92 lookup main
512:	from all to 192.168.9.146 lookup main
512:	from all to 192.168.5.21 lookup main
512:	from all to 192.168.25.41 lookup main
512:	from all to 192.168.26.54 lookup main
512:	from all to 192.168.2.30 lookup main
512:	from all to 192.168.27.246 lookup main
512:	from all to 192.168.21.133 lookup main
1024:	from all fwmark 0x80/0x80 lookup main
1536:	from 192.168.31.149 lookup 2
1536:	from 192.168.12.92 lookup 2
1536:	from 192.168.9.146 lookup 2
1536:	from 192.168.5.21 lookup 2
1536:	from 192.168.25.41 lookup 3
1536:	from 192.168.26.54 lookup 3
1536:	from 192.168.2.30 lookup 3
1536:	from 192.168.27.246 lookup 3
1536:	from 192.168.21.133 lookup 3
1536:	from 20.0.49.215 lookup 2
32766:	from all lookup main
32767:	from all lookup default
a2ush added a commit to a2ush/amazon-vpc-cni-k8s that referenced this pull request Mar 12, 2022
@a2ush
Modify the example comment along the current behavior.
e236a33 
From v1.8.0, amazon-vpc-cni-k8s no longer sets VPC CIDR to the from-pod rule.
aws#1475
Ex)
0:	from all lookup local
512:	from all to 192.168.13.238 lookup main
512:	from all to 192.168.16.73 lookup main
512:	from all to 192.168.17.61 lookup main
512:	from all to 192.168.23.14 lookup main
512:	from all to 192.168.25.98 lookup main
512:	from all to 192.168.31.149 lookup main
512:	from all to 192.168.12.92 lookup main
512:	from all to 192.168.9.146 lookup main
512:	from all to 192.168.5.21 lookup main
512:	from all to 192.168.25.41 lookup main
512:	from all to 192.168.26.54 lookup main
512:	from all to 192.168.2.30 lookup main
512:	from all to 192.168.27.246 lookup main
512:	from all to 192.168.21.133 lookup main
1024:	from all fwmark 0x80/0x80 lookup main
1536:	from 192.168.31.149 lookup 2
1536:	from 192.168.12.92 lookup 2
1536:	from 192.168.9.146 lookup 2
1536:	from 192.168.5.21 lookup 2
1536:	from 192.168.25.41 lookup 3
1536:	from 192.168.26.54 lookup 3
1536:	from 192.168.2.30 lookup 3
1536:	from 192.168.27.246 lookup 3
1536:	from 192.168.21.133 lookup 3
32766:	from all lookup main
32767:	from all lookup default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@achevuru achevuru achevuru approved these changes

@jayanthvn jayanthvn jayanthvn approved these changes

@anguslees anguslees anguslees approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Traffic from outside VPC does not reach pod
4 participants
@kishorj @jayanthvn @anguslees @achevuru